-
Notifications
You must be signed in to change notification settings - Fork 48
#1653 implementation get version and get edition docker #1668
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
#1653 implementation get version and get edition docker #1668
Conversation
…plementation-get-version-and-get-edition-docker
Pull Request Test Coverage Report for Build 20716812541Details
💛 - Coveralls |
cli/src/main/java/com/devonfw/tools/ide/tool/docker/Docker.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/tool/docker/Docker.java
Outdated
Show resolved
Hide resolved
hohwille
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hkosim thank you very much for your PR. You did solid analysis, testing and fixing of the problem. Your code looks good. 👍
I will merge after accepting the suggestions. I will also do polishing and adding a test after the merge in a separate PR to get this merged.
cli/src/main/java/com/devonfw/tools/ide/tool/docker/Docker.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/tool/kubectl/KubeCtl.java
Outdated
Show resolved
Hide resolved
Wow. CI is unable to download public artifact from maven central and gets 403... What is wrong here and why now? Temporary bug of maven central? |
|
Error seems currently reproducible (I restarted the build and it failed with the same error). |
|
I deleted the maven-parent 14 pom in my local repo and rebuild IDEasy with maven:
So the download of the POM works. |
Line 5 in 8cc587d
So the credentials only apply for servers with id repository and IMHO the default maven download has id central.Still does not make any sense at all. |
|
I triggered a build in PR #1650 and it was green so it is not a general maven download issue in CI. |
This PR fixes #1653
Implemented changes:
getVersion()andgetEdition()forDockeras well askubectlis now working properly on Windows and Linux.Note:
regand checks for installation in windows registryaptand filtered usinggrepPseudocode:
FUNCTION getInstalledVersion():
FUNCTION getInstalledEdition():
Findings about Testcases
Testcases was not yet implemented correctly. The idea that has been tested:
What could work:
DockerTest.javawith path for mockedrdctland bash path* on customenvironment.properties(*hard-coded) .-> getVersion() is working properly.
Still in progress:
Mocking
cmdandregfor checking DockerDesktop version.regmight also be mocked with file with.cmdextension.Checklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internal